Skip to content

gh-131798: JIT inline function addresses of builtin methods #146906

Open
kumaraditya303 wants to merge 17 commits intopython:mainfrom
kumaraditya303:jit-inline-funcptr
Open

gh-131798: JIT inline function addresses of builtin methods #146906
kumaraditya303 wants to merge 17 commits intopython:mainfrom
kumaraditya303:jit-inline-funcptr

Conversation

@kumaraditya303
Copy link
Copy Markdown
Contributor

@kumaraditya303 kumaraditya303 commented Mar 31, 2026

@Fidget-Spinner
Copy link
Copy Markdown
Member

Fidget-Spinner commented Mar 31, 2026

Edit: I might be wrong on the following, so please ignore the rest.

Really sorry I just realised this optimization may not be sound:

The builtin call specializations only check for that the builtin tp flags match, I don't think they check for tp_version_tag and such. So we can have a method that changes its tp_slots under our feet.

Sorry for taking up your time!

@Fidget-Spinner
Copy link
Copy Markdown
Member

Fidget-Spinner commented Mar 31, 2026

The rewriting that we discussed might still pay off though (rewriting {}.get() to a custom uop for example).

That should also be sound, provided we have the correct guards.

@Fidget-Spinner
Copy link
Copy Markdown
Member

Fidget-Spinner commented Apr 3, 2026

I think this is safe: let me explain why:

  1. PyMethodDescrObjects are immutable (as Kumar pointed out).
  2. type version tags are unique (unless they overflow, which is pretty unlikely as they have limits now).
  3. _GUARD_TYPE_VERSION and the type watcher later implies the type and thus method transitively later on.
    TLDR: This is safe on the invariant that LOAD_ATTR guards itself, and all known information is sourced from there to optimize the calls.

@kumaraditya303
Copy link
Copy Markdown
Contributor Author

kumaraditya303 commented Apr 3, 2026

Microbenchmark results of calling bunch of methods on each type:

Type this branch main branch speedup
list 29,186 31,082 +6.5%
dict 18,244 18,761 +2.8%
set 9,152 9,681 +5.8%
str 18,185 18,922 +4.1%
deque 19,127 21,072 +10.2%

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants